backend/tests/apps/api/rest/v0/chapter_test.py (1)

35-35: Consider testing with populated leaders.

The test only verifies the empty list case. Add a test case with actual leader names to ensure the resolver correctly maps leaders_raw to the leaders field.

Example test data:

{
    ...
    "leaders_raw": ["Alice Smith", "Bob Jones"],
}

Then assert:

assert chapter.leaders == ["Alice Smith", "Bob Jones"]

backend/tests/apps/api/rest/v0/project_test.py (1)

35-35: Consider testing with populated leaders.

Similar to the chapter test, consider adding a test case with actual leader names to ensure the resolver correctly maps leaders_raw to the leaders field.

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

backend/tests/apps/api/rest/v0/project_test.py (2)

30-32: Consider extracting MockEntityMember to a shared test utility.

The MockEntityMember class is duplicated in both chapter_test.py and project_test.py. Extracting it to a common test fixture or utility module would reduce duplication and make future maintenance easier.

Example approach:

Create backend/tests/apps/api/rest/v0/conftest.py or a test utilities module:

class MockEntityMember:
    def __init__(self, name):
        self.member_name = name

Then import and use it in both test files.

---

39-39: Consider adding test case for empty leaders list.

The current test only validates the happy path with two leaders. Adding a test case with an empty entity_leaders list would improve coverage and ensure the resolver handles edge cases correctly.

Example:

def test_project_serializer_with_no_leaders():
    class MockEntityMember:
        def __init__(self, name):
            self.member_name = name
    
    class MockProject:
        def __init__(self, data):
            for key, value in data.items():
                setattr(self, key, value)
            self.nest_key = data["key"]
            self.entity_leaders = []  # Empty leaders list
    
    project_data = {
        "created_at": "2023-01-01T00:00:00Z",
        "description": "A test project",
        "key": "test-project",
        "level": "other",
        "name": "Test Project",
        "updated_at": "2023-01-02T00:00:00Z",
    }
    project = ProjectDetail.from_orm(MockProject(project_data))
    assert project.leaders == []

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

backend/apps/api/rest/v0/project.py (1)

45-48: Extract duplicated resolver logic to a shared helper.

The resolve_leader_names method is identical to the one in backend/apps/api/rest/v0/chapter.py (lines 47-49). This duplication violates DRY principles and increases maintenance burden.

Consider extracting this logic to a shared utility function or mixin that both ProjectDetail and ChapterDetail can use. For example:

# In a shared utils module (e.g., apps/api/rest/v0/utils.py)
def resolve_entity_leader_names(obj):
    """Resolve leader names from entity_leaders relationship."""
    return [leader.member_name for leader in obj.entity_leaders]

Then use it in both schemas:

     @staticmethod
     def resolve_leader_names(obj):
         """Resolve leader names."""
-        return [leader.member_name for leader in obj.entity_leaders]
+        from apps.api.rest.v0.utils import resolve_entity_leader_names
+        return resolve_entity_leader_names(obj)

backend/tests/apps/api/rest/v0/project_test.py (2)

30-39: Extract duplicated mock class to a shared test fixture.

The MockEntityMember class is identical to the one in backend/tests/apps/api/rest/v0/chapter_test.py (lines 33-35). This duplication increases maintenance overhead and violates DRY principles.

Consider creating a shared test fixture or conftest.py:

# In backend/tests/apps/api/rest/v0/conftest.py or a shared fixtures module
import pytest

class MockEntityMember:
    """Mock for entity member used in API serializer tests."""
    def __init__(self, name):
        self.member_name = name

@pytest.fixture
def mock_entity_members():
    """Fixture providing sample entity members."""
    return [MockEntityMember("Alice"), MockEntityMember("Bob")]

Then simplify both test files:

def test_project_serializer_validation(project_data, mock_entity_members):
    class MockProject:
        def __init__(self, data):
            for key, value in data.items():
                setattr(self, key, value)
            self.nest_key = data["key"]
            self.entity_leaders = mock_entity_members
    # ... rest of test

---

46-46: Add edge case tests for leader_names.

The test only covers the happy path with two leaders. Consider adding test cases for:

• Empty entity_leaders list
• Projects with no leaders assigned
• Leaders with missing or None member_name values

Add additional test cases:

def test_project_serializer_with_no_leaders():
    """Test serialization when project has no leaders."""
    class MockProject:
        def __init__(self):
            self.created_at = "2023-01-01T00:00:00Z"
            self.description = "Test project"
            self.nest_key = "test-key"
            self.level = "incubator"
            self.name = "Test"
            self.updated_at = "2023-01-02T00:00:00Z"
            self.entity_leaders = []
    
    project = ProjectDetail.from_orm(MockProject())
    assert project.leader_names == []

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2223
File: backend/apps/owasp/models/common.py:213-232
Timestamp: 2025-09-06T19:03:01.985Z
Learning: In the OWASP Nest project, the get_leaders_emails() method in RepositoryBasedEntityModel is designed to only capture leaders with mailto: links from leaders.md files, intentionally ignoring plain text names without email addresses. The current regex implementation works correctly for the intended behavior as validated by comprehensive test cases.

Learnt from: kart-u
Repo: OWASP/Nest PR: 3101
File: backend/apps/common/extensions.py:92-98
Timestamp: 2025-12-31T05:17:39.659Z
Learning: In this codebase, import OperationType for GraphQL operations from the graphql-core package rather than from strawberry. Use 'from graphql import OperationType'. Strawberry re-exports via graphql-core internally, so relying on strawberry's API may be brittle. Apply this rule to all Python files that deal with GraphQL operation types; ensure imports come from graphql (graphql-core) and not from strawberry packages. This improves compatibility and avoids coupling to strawberry's internals.

Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:41-47
Timestamp: 2026-01-01T17:48:23.963Z
Learning: In Django code, be aware that a QuerySet's boolean evaluation (e.g., if not queryset) runs a database query to determine emptiness. While it is technically valid to use the queryset in a boolean context, use queryset.exists() for existence checks to avoid unnecessary queries and improve performance. Applicable broadly to Python/Django files rather than just this specific path.

Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2606
File: backend/apps/api/rest/v0/project.py:43-48
Timestamp: 2025-11-23T11:37:26.253Z
Learning: In the OWASP Nest backend, `entity_leaders` is a `property` method defined in `RepositoryBasedEntityModel` (backend/apps/owasp/models/common.py) that returns a dynamically constructed QuerySet. It cannot be prefetched using standard `prefetch_related()` because Django's prefetch mechanism only works on model fields and relations, not property methods.